Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a more robust solution for handling race conditions in user creation by improving the createUser function. The changes focus on preventing duplicate key violations that can occur when multiple concurrent requests attempt to create the same user.
Key changes:
- Enhanced database-level conflict handling with proper upsert logic
- Added comprehensive documentation for the race condition handling approach
- Simplified database queries by removing unnecessary compound conditions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/app/src/lib/auth.ts | Added explanatory comments about race condition handling in user creation |
| packages/app/src/db/crud/user.ts | Completely refactored createUser function with robust upsert logic and comprehensive documentation |
| .returning() | ||
| .onConflictDoUpdate({ | ||
| target: [user.email], | ||
| target: [user.authId], |
There was a problem hiding this comment.
The conflict target has been changed from [user.email] to [user.authId], but the code also handles cases where users exist by email but not by authId (lines 70-94). This creates a logical inconsistency where the upsert won't handle email conflicts, potentially causing unique constraint violations on the email field if it has a unique constraint.
| target: [user.authId], | |
| target: [user.authId, user.email], |
| .returning() | ||
| .execute(); | ||
|
|
||
| return updatedUser; |
There was a problem hiding this comment.
The function updates an existing user's authId when found by email (lines 78-93), but this could lead to security issues. If two different authentication providers return the same email address, this code would reassign the user account to the new authId, potentially giving unauthorized access to another user's account.
| termsOfUseAcceptedAt: userInsert.termsOfUseAcceptedAt, | ||
| updatedAt: new Date(), | ||
| }, | ||
| }) |
There was a problem hiding this comment.
The comment mentions handling 'remaining race conditions' but the earlier logic already handles user existence by both authId and email. This upsert clause may be redundant given the explicit checks above, making the code unnecessarily complex.
No description provided.